Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce .NET 8 SDK for new C# modules #2046

Merged
merged 3 commits into from
Jan 7, 2025
Merged

Enforce .NET 8 SDK for new C# modules #2046

merged 3 commits into from
Jan 7, 2025

Conversation

RReverser
Copy link
Member

@RReverser RReverser commented Dec 10, 2024

Description of Changes

This fixes an issue where users with .NET 9 SDK installed can't build C# modules even if .NET 8 SDK is installed as well.

Unfortunately, this covers only new projects created with spacetime init as that's the only place where we control the project directory. For everyone else, we print instructions with a link to the official configuration docs.

API and ABI breaking changes

If this is an API or ABI breaking change, please apply the
corresponding GitHub label.

Expected complexity level and risk

How complicated do you think these changes are? Grade on a scale from 1 to 5,
where 1 is a trivial change, and 5 is a deep-reaching and complex change.

This complexity rating applies not only to the complexity apparent in the diff,
but also to its interactions with existing and future code.

If you answered more than a 2, explain what is complex about the PR,
and what other components it interacts with in potentially concerning ways.

Testing

Have both .NET 8 and .NET 9 installed globally (e.g. latest Visual Studio includes .NET 9 SDK by default, and you can install .NET 8 SDK manually from the official website).

Try to init a new Spacetime C# module and publish it. Before this PR, it will fail, with this PR it will work.

I have run the above testing manually.

Copy link
Contributor

@cloutiertyler cloutiertyler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tested this? Could you please update the testing section of the PR template?

crates/cli/src/subcommands/init.rs Outdated Show resolved Hide resolved
@RReverser
Copy link
Member Author

Have you tested this? Could you please update the testing section of the PR template?

Yeah, without this PR publishing C# modules stopped working altogether after Visual Studio update. Updated the testing section.

@bfops bfops added the release-any To be landed in any release window label Dec 30, 2024
@cloutiertyler
Copy link
Contributor

@RReverser could you just confirm that you ran the tests in the testing section?

@RReverser
Copy link
Member Author

@RReverser could you just confirm that you ran the tests in the testing section?

Yes. Without this fix I can't publish modules on my machine since I installed .NET 9 (at least without manual workarounds), but with this PR I can - at least for the new modules.

This fixes an issue where users with .NET 9 SDK installed can't build C# modules even if .NET 8 SDK is installed as well.

Unfortunately, this covers only new projects created with `spacetime init` as that's the only place where we control the project directory. For everyone else, we print instructions with a link to the official configuration docs.
Copy link
Contributor

@cloutiertyler cloutiertyler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that you have run the tests specified in the testing section, I will approve this PR.

@RReverser RReverser added this pull request to the merge queue Jan 7, 2025
Merged via the queue into master with commit 763ba5e Jan 7, 2025
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-any To be landed in any release window
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants